Remove razor #425
Remove razor #425
Conversation
|
We should probably do this. Thanks for looking into this! Change looks ok to me, I think we can do better on the generator type definition. Maybe with type abbreviations or records? |
|
It's great to see this happening. I think having "core library" with additional layers is a really good strategy. I think I think it would be much nicer to write: This is a more significant change - because all methods now need to return result (and we need to define nice types for this result) rather than calling generator on their own. But I think, in the long run, this really enables more uses of the library. Callbacks are framework smell :-). That said, I'm happy to accept the PR as it is, because I think it's a good move. |
|
As suggested by @matthid - using record for generator function input. |
|
MetadataFormat returns F# record (as Tomas prefers ;) ). Will do the same for Formatting tomorrow :) |
|
OK... I think it's ready Fixed CommandLineTool and Tests by using Razor API build on top of my changes (it has exactly the same API as previously Core functionality). I guess in the future someone should move changed tests to Razor.Test project and add true unit tests for Core API. |
|
Hmmm... looks like it's passing tests but failing on |
|
@tpetricek @matthid any chance one of you could take a look at this? I tried to figure out why the build was failing but I had no luck with it. I know there have been issues with the VFPT core in the past, but I can publish tailored versions of it to https://ci.appveyor.com/nuget/fsharp-editing like I did to get the latest version using FCS 8.0.0 in https://github.com/Krzysztof-Cieslak/FSharp.Formatting/pull/1/commits It'd be great if we could get FSharp.Formatting ready to run on netcore, upgrade the framework targets to 4.6.1, adjust the src for api compatibility, etc. I can take care of whatever you guys need on the VFPT side. |
|
Problem was that we now have a single instead of two caches, and While strictly speaking it would be better to leave the exception and add the optional Anyway it was good to notice (and fix) this breaking change. |
| XmlMemberMap = map; | ||
| MarkdownComments = markDownComments; | ||
| UrlMap = urlMap; | ||
| UrlRangeHighlight = urlRangeHighlight; |
There was a problem hiding this comment.
As you already did some cleanup we might want to get rid of the semicolons as well :)
| // let ty = v.LogicalEnclosingEntity.ReflectionType | ||
| // if ty.IsGenericType then ty.GetGenericArguments().Length | ||
| // else 0 | ||
| //else |
There was a problem hiding this comment.
@tpetricek do you remember this code?
| ?moduleTemplate = moduleTemplate, ?typeTemplate = typeTemplate, ?xmlFile = xmlFile, ?sourceRepo = sourceRepo, ?sourceFolder = sourceFolder, | ||
| ?publicOnly = publicOnly, ?libDirs = libDirs, ?otherFlags = otherFlags, ?markDownComments = markDownComments, ?urlRangeHighlight = urlRangeHighlight, ?assemblyReferences = assemblyReferences) | ||
| ( Seq.singleton dllFile, ?parameters = parameters, ?xmlFile = xmlFile, ?sourceRepo = sourceRepo, ?sourceFolder = sourceFolder, | ||
| ?publicOnly = publicOnly, ?libDirs = libDirs, ?otherFlags = otherFlags, ?markDownComments = markDownComments, ?urlRangeHighlight = urlRangeHighlight) |
There was a problem hiding this comment.
@Krzysztof-Cieslak
Could we provide the "old" overloads via extensions so nothing breaks (source code compat is probably good enough as this is mostly used by scripts)?
|
Thanks a lot for merging, sorry I haven't done last fixes / suggestion but I'm pretty busy with some other stuff recently. |
|
@Krzysztof-Cieslak No problem, let me know if you need a release (for example to play with Fornax) ;) |
|
@matthid you're my hero 🎊 |
|
really @Krzysztof-Cieslak is the hero here ... |
|
he already knows how much I appreciate this 😉 |
Trying to remove Razor dependency from core of F#.Formatting, and then add same functionality on top of it. It will enable using different rendering systems in the future.
Targeting
version3branch, as it'll contain breaking changes.